-
Notifications
You must be signed in to change notification settings - Fork 537
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add support for hard/soft bloom filter base + stash #22828
Conversation
1c5160b
to
aeb1b36
Compare
f30bf73
to
f387a3d
Compare
f39e370
to
08d3851
Compare
779be6e
to
b2007e3
Compare
we should almost never fix two issues with a single PR, so please fix mozilla/addons#15166 in a different PR. |
227f07e
to
3917ee4
Compare
I've never heard of this rule and regularly do this. Why should that be a rule? |
3917ee4
to
dc6f9cb
Compare
5492d61
to
598ef06
Compare
598ef06
to
d1c4088
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am done for today, but that makes it much easier to see what's going on, thanks.
To be continued...
c357bc7
to
5379148
Compare
99a0e05
to
3c32be4
Compare
7cfa66a
to
fb2db99
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some more feedback. This is coming together.
4e83dcf
to
8c0030a
Compare
Remove unecessary and redundant code Fix ordering of cache/stash + increase validity of tests Upload multiple filters More logs + correct handling of attachment_type Verify cron passes correct args to task TMP: Ignore soft blocks Add waffle switch Fix invalid class reference Update to correct waffle switch Update to fix the test reafactoring add missing tests Apply suggestions from code review Co-authored-by: William Durand <[email protected]> Updates from review Ensure blocks of type X are excluded from stash if filter of type X is also being uploaded TMP: squash Better exclusion of stashes from updated filters + more comment resolution Delete correct files from remote settings: - delete any existing attachments matching filters we are reuploading - delete any stashes that are older than the oldest filter Add tests for shape of stash Add test for the craziness that is stash Simple is better than complex Compare to base filter when determining stash Use block type level base filter Apply suggestions from code review Co-authored-by: William Durand <[email protected]>
faa29ba
to
c02eff7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
r+wc, thanks!
(note: it'd be good if you could clear the commit message before merging) |
Co-authored-by: William Durand <[email protected]>
0a13ed7
to
67be5ff
Compare
Fixes: mozilla/addons#15014
Relates to: mozilla/addons#15155
Description
Adds logic to generate and write bloom filters for both soft and hard blocked addons. Additionally this PR introduces logic to determine whether we should update one or both bloom filters and or a stash as multiple possible outcomes are possible now. Finally, we handle cleaning up files on a more granular level from both the local storage and remote settings.
Context
Now when we run the
upload_mlbf_to_remote_settings
cron job we will check for both hard and soft blocked items. It is possible to:This adds a bit of complexity we need to address.
Additionally, instead of deleting all records from remote settings, we need to check for the current set of block filters and only delete records older than the older of the two.
Finally, since it is also possible to run the cron when no updates have occurred, we can safely delete mlbf cache files when that happens as there is no benefit from diffing an empty array.
Testing
This is gonna suck to test. First some preparation work.
Setup
enable-soft-blocking
andblocklist_mlbf_submit
waffle switchsrc/olympia/constants/blocklist.py
See the test scenarios
Now you can call the
_blocked_addon
method to create an addon with block/version of the specified type.Ex:
If you run the cron job now, you'd expect a blocked filter and a stash with the soft blocked version added.
Checklist
#ISSUENUM
at the top of your PR to an existing open issue in the mozilla/addons repository.